-
-
Notifications
You must be signed in to change notification settings - Fork 718
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dask internal inherit config #4364
Dask internal inherit config #4364
Conversation
CI failures seem to be unrelated (they are the same on master) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not remember exactly why we gave priority to the existing global config. My guess is that we didn't want to overwrite an option given specifically to a worker. I don't know if that is an issue?
Thanks @madsbk. From my reading of the code I don't think this would be an problem. It actually appears that the worker gets some values from the config before this is applied. So it may be that today some of these values are missed. I wonder if upstreaming this to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if upstreaming this to dask.config all together would be the best approach?
I think it is fine to merge this for now.
Thanks @madsbk |
Setting
priority="old"
when updating config withDASK_INTERNAL_INHERIT_CONFIG
means that default config options can never be overridden.When used in
SSHCluster
to package up the local config to send to the scheduler and workers any value which has a default set indistributed.yaml
will be returned to the default.This change resolved my issue, however I'm guessing this option was included for a reason. @madsbk do you know why this was added?
Closes #4363
I also added a test for the serialization as I ended up writing one as part of putting together the MVRE for #4363.